perf(runtime-core): improve efficiency of normalizePropsOptions #11409
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #9739.
The result of
normalizePropsOptions
is cached, so the performance usually doesn't matter too much. But there are two cases where the cache may not help:<script setup>
block of another component.A lot of time in
normalizePropsOptions
is spent ingetType
, trying to figure out the values of theshouldCast
andshouldCastTrue
flags. I go into a bit more detail about that at #9739 (comment).The Playground isn't necessarily the best tool for benchmarking, but here's an example to try to illustrate that my changes have improved the performance:
I based my branch on 3.4.33, so it should be a fair comparison. It seems to be faster in both Chrome and Firefox (I didn't try other browsers).
The Playground example isn't very 'real'. It's designed to focus on the performance of
normalizePropsOptions
as much as possible, so the performance benefit would be much less in real applications. But if performance can be improved without hurting bundle size or maintainability, it seems worth it to me.The Playground uses small arrays for the
type
. I think that's representative of real code. I didn't think it would be realistic to test with arrays of hundreds or thousands of items, so my conclusions are all based on the assumption that the arrays will be relatively short.Most of my benchmarking I did using Node instead, calling
normalizePropsOptions
directly. Using a similarprops
definition to the one in the Playground above I saw an improvement of about 35%. There's a version of the code I used at:That's using arrays for the prop types. I also tested using other values, such as
type: String
. The performance still seemed slightly better, maybe about 10% better, but it was less clear relative to the background noise in the measurements.I also tried to reduce the size of the build. There was a lot of code involved in performing the
shouldCast
/shouldCastTrue
checks, much of which is no longer needed. I experimented with various implementations, testing both the performance and build size. The first commit on this PR, 3f37cf7, gave a slightly smaller build size (saving 176 bytes rather than 116), but it wasn't quite as fast as the second version (about 30% faster than 3.4.33, rather than 35%).The original motivation for these changes came from #9739, which proposed adding a cache to
getType
. Since that PR was opened there have been other changes togetType
via #10327. I think those other changes make adding a cache less effective. The remaining problem is just thatgetType
gets called so much, which is essentially the problem I'm attempting to address here.One of the changes introduced by #10327 was this code:
core/packages/runtime-core/src/componentProps.ts
Lines 610 to 614 in 422ef34
I'm not entirely sure why this code was added. It isn't explained in the PR description and there aren't any tests for it. I suspect it may have been introduced by mistake. It leads to strange handling of some edge cases. For example, consider a prop with
type: [new Boolean(true)]
. Obviously that's nonsense, but as of 3.4.19 (which is the first release to include that change) it behaves the same astype: Boolean
. Playground example.In the changes I've made in this PR I have not attempted to retain this behaviour. I'm now just checking for
isFunction(type) && type.name
, which seems to be all we need in practice.My code does duplicate this check, once for array types and then again for non-arrays. I did try to remove the duplication, but it just ended up adding extra bytes to the build, or making it slower, or both. To see an example of one of my attempts, take a look at the first commit on this PR, 3f37cf7. That does remove the duplication, but at the expense of being slower. I also felt that version was slightly more difficult to understand for future contributors.
I've made a change to
NormalizedProp
to removenull
from the type. From what I can tell, thenull
was needed when that type was first introduced, but other code has changed since, so it can now be removed. Removing thenull
allowed me to remove theif (prop)
check innormalizePropsOptions
. While this doesn't really impact the performance, it did seem to be redundant and shaves a few bytes off the bundle.The main change is to introduce a single
for
loop over thetype
array. The type is then checked directly against the strings'Boolean'
and'String'
, rather than using multiple calls togetType
to get those strings. ThegetType
function is no longer used here, and is now only used in dev code, allowing it be to treeshaken.I experimented with using other types of loop, such as
some
andfor
/of
, but ultimately a basicfor
seemed to give the best performance. In particular, usingfor
/of
seemed slightly slower in Firefox. The superior performance of a basicfor
is maybe no great surprise, but nevertheless I did try to confirm that it really was best when working with short arrays in current browsers.Slight tangent...
I find the Brotli sizes to be a bit erratic. They seem to jump around almost randomly. For example, at the time of writing, the Brotli/overall value given in the size report is +2. But if I swap round these two lines to be in the opposite order, it jumps to -41:
I gave up chasing the Brotli size and chose to focus on Size and Gzip instead, as they seemed like a more reliable way to judge progress.